Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(MRM_handler, MRM_emergency_stop_operator): revert mrm_stop parameter, enable mrm_comfortable_stop #1265

Merged
merged 1 commit into from
Dec 12, 2024
Merged

feat(MRM_handler, MRM_emergency_stop_operator): revert mrm_stop parameter, enable mrm_comfortable_stop #1265

merged 1 commit into from
Dec 12, 2024

Conversation

yuki-takagi-66
Copy link
Contributor

@yuki-takagi-66 yuki-takagi-66 commented Dec 9, 2024 •
edited
Loading

Description

This PR contains the following changes.

  1. Revert the mrm_emergency_stop parameter, which was changed by feat(autonomous_emergency_braking): enable AEB stop in vehicle_cmd_gate and diag_graph_agg #1099 half a year ago. These changes are oriented to AEB; however, these parameters are used for all MRM emergency stops. Therefore, I propose to revert these parameters to maintain any behavior other than AEB.
  2. Enabling comfortable_stop. MRM comfortable stop was tested enough by tier4 operations. Now I recommend enabling it as public.

How was this PR tested?

psim test

Notes for reviewers

None.

Effects on system behavior

None.

@yuki-takagi-66
po
b998514 
Signed-off-by: yuki-takagi-66 <[email protected]>
@yuki-takagi-66 yuki-takagi-66 marked this pull request as ready for review December 9, 2024 07:30
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 9, 2024

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@yuki-takagi-66 yuki-takagi-66 marked this pull request as draft December 9, 2024 07:31
@yuki-takagi-66 yuki-takagi-66 marked this pull request as ready for review December 12, 2024 10:14
@isamu-takagi
Copy link
Contributor

These changes are oriented to AEB; however, these parameters are used for all MRM emergency stops. Therefore, I propose to revert these parameters to maintain any behavior other than AEB.

@yuki-takagi-66 Can I assume that AEB will still operate safely if I change the parameters back?

@yuki-takagi-66
Copy link
Contributor Author

These changes are oriented to AEB; however, these parameters are used for all MRM emergency stops. Therefore, I propose to revert these parameters to maintain any behavior other than AEB.

@yuki-takagi-66 Can I assume that AEB will still operate safely if I change the parameters back?

@isamu-takagi
Yes, AEB works properly even if we change the parameters back.
Note that, the complete safety is not our (OSS) aims and this is not achieved by the both parameters.

isamu-takagi
isamu-takagi approved these changes Dec 12, 2024
View reviewed changes
Copy link
Contributor

@isamu-takagi isamu-takagi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (After speaking with the people involved, I learned that they had changed the parameters in order to pass the tests, but the conditions were too strict, so they decided that the previous parameters, which had been confirmed to work, would be better.

@yuki-takagi-66 yuki-takagi-66 merged commit 1ded9c3 into autowarefoundation:main Dec 12, 2024
22 checks passed
@yuki-takagi-66 yuki-takagi-66 deleted the takagi/mrm/update-settings branch December 12, 2024 11:04
@tier4-autoware-public-bot tier4-autoware-public-bot bot mentioned this pull request Dec 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@isamu-takagi isamu-takagi isamu-takagi approved these changes

@ito-san ito-san Awaiting requested review from ito-san ito-san is a code owner

Assignees

@isamu-takagi isamu-takagi

Labels
run:build-and-test-differential
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yuki-takagi-66 @isamu-takagi